Please don't skip tests - which of these are valid/invalid? If a different analyzer is catching these errors now, would it make sense to update the test to detect both?

All of these cases, we don't check them anymore. ANY method without valid return type, analyzer just skips check. So these tests are invalid until we find a better way to tell what methods we want to check. This is what you brought up, and @m-nash agrees with you.

Hm, I'm confused. Isn't AsyncPageable a valid return type? The analyzer should be checking that we have sync and async variants of methods returning pageable.

Would it be possible to separate out the tests return invalid types for service methods from the ones that are testing valid return types?

It looks like the result of skipping both these tests is that we don't have any tests for AZ0015, which isn't a good place for us to be for this analyzer.

AZ0015 just not effective now.

Ah, I didn't realize that implementing it this way made AZC0015 completely non-functional.

I think AZC0015 is a useful diagnostic error to have running in azure-sdk-for-net. For example, if I write a method that accidentally returns Task<Model> where I meant to return Task<Response<Model>>, I would like a warning to help me find this, and not rely on a human architect catching my mistake.

Is it possible to change the implementation for AZC0018 and AZC0019 so that AZC0015 is still functional? I realize that a lot of services have opted-out of this warning, we should see if we can remove these where it's possible, but that doesn't have to be part of this PR.

Why is this removed?

Because we change the logic to detect ambiguity. AssetNotNull is not needed any more to detect ambiguity. Current logic is in https://github.com/Azure/azure-sdk-for-net/pull/37795/files

Curious: why make this sealed?

It was sealed before. I removed it in my previous PR to do some change. Now this change is revert so the sealed need to be added back.

Curious: does this enable AZC0015 for these tests?

It just means we don't need to suppress AZC0015. That's it.

AZC0015 is not effective now.

Are we still able to detect and verify that subclient methods need to be virtual?

(Being virtual is required for mocking, so we need to be able to enforce it for subclient methods as well)

Two things here:

1. I'm not deleting anything. I think you should see all the comments together, not just one iteration.
2. Subclient cannot be checked any more because its return not is not one of Operation, Pageable, Response.

Nit: is the name of this method NotProduced or Produced?

I think it should be Produced. I'm just adding back the deleted one which called itself NProduced. Updated to Produced.

Curious: why is this being removed?

I'm not removing. I think you are not seeing the right iteration.